-
-
Notifications
You must be signed in to change notification settings - Fork 942
Don't return invalid region from lsp-semantic-tokens--fontify. #4876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Currently it returns 0 . 0 which is outside of the buffer, (point-min) is 1. It's better to return nil instead because such invalid region might break other packages, i.e. indent-bars.
|
It also needs to call the downstream function no matter what. These functions should only expand, not contract (or zero out) the indicated region. |
But then the downstream function will colorize that region, won't it? This will result in inconsistent syntax highlighting and flickering once lsp-mode gets semantic tokens from lsp-server. We don't really want to get colors from the major mode here. |
Yes, but otherwise you are "hogging" jit-lock all for yourself, and other packages which need access to jit-lock updates simply won't work. The way eglot semantic and semel are solving this is to decorate text with semantic-based text-properties as/when available, then, in font-lock (just font-lock-keywords as Stefan is recommending) turn those into 'face. So if there is no semantic info yet available, font-lock wins, but once available, lsp wins. Honestly, I don't think lsp should need to advise Note that |
|
sorry, I just read this today; sounds like I was using the wrong approach but if I can get rid of having to interfere with |
|
@kurnevsky I pushed a commit to my fork at sebastiansturm/lsp-mode that uses font-lock-keywords. Like @jdtsmith said, this seems to be much simpler and work at least as well. But I haven't tested this much and have only read some of the docs on font-lock-keywords so when I find the time next week I should probably do some more poking around before I open a PR with this. But if you can test this and check whether it solves your current issue, that'd be much appreciated |
|
@sebastiansturm just tested your changes, it solves the problem with indent-bars :) |
|
thanks :) I'll open a PR then (not today but I think I should get to it over the weekend) |
Currently it returns 0 . 0 which is outside of the buffer, (point-min) is 1. It's better to return nil instead because such invalid region might break other packages, i.e. indent-bars.
See jdtsmith/indent-bars#117